src/io/cache_io/page.h:62

• Page::SetPageSize mutates a process-wide page size after pages may already have been allocated. Since Page instances don’t store their own allocation size, changing page_size_ can make later reads/copies compute offsets using a new size against buffers allocated with an old size, leading to out-of-bounds memory access/corruption (e.g., multiple CacheIO instances with different page sizes, or tests changing page size between runs). Consider making page size immutable (set-once) or storing the page size per Page/CacheIO instance instead of a mutable static.

    static void
    SetPageSize(uint64_t page_size) {
        static std::mutex mtx;
        std::lock_guard<std::mutex> lock(mtx);
        page_size_ = page_size;
    }

src/io/cache_io/cache_io.h:76

• WriteImpl updates the inner IO but doesn’t invalidate cached pages that overlap the written range. If the same region was previously read and cached, subsequent reads can return stale data. Unless the entire library guarantees non-overlapping read/write regions, it’s safer to evict affected pages on write to preserve read-after-write coherence.

    void
    WriteImpl(const uint8_t* data, uint64_t size, uint64_t offset) {
        inner_io_->WriteImpl(data, size, offset);
        if (offset + size > this->size_) {
            this->size_ = offset + size;
        }
    }

src/io/cache_io/cache_io.h:117

• The PR description states DirectRead should return an internal cache pointer for a single-page hit (need_release=false), but this implementation always allocates/copies and sets need_release=true. Also, returning a raw pointer into a cached page would require a pin/unpin mechanism to keep the Page alive until Release(); otherwise eviction could free the page while the caller still uses the pointer. Either update the PR description to match the safe allocate/copy behavior, or introduce a pinned-handle design that makes returning internal pointers safe.

    [[nodiscard]] const uint8_t*
    DirectReadImpl(uint64_t size, uint64_t offset, bool& need_release) const {
        need_release = false;
        if (size == 0 or not is_valid_range(size, offset)) {
            return nullptr;
        }
        auto* data = reinterpret_cast<uint8_t*>(this->allocator_->Allocate(size));
        if (data == nullptr) {
            return nullptr;
        }
        if (not ReadImpl(size, offset, data)) {
            this->allocator_->Deallocate(data);
            return nullptr;
        }
        need_release = true;
        return data;
    }

src/io/cache_io/cache_io.h:55

• CacheIOParameter exposes eviction_strategy_, but CacheIO currently always constructs LRUPageCache and never consults the parameter. This makes the configuration knob misleading and doesn’t match the PR description’s “pluggable eviction strategy” design. Consider either wiring strategy selection in CacheIO (even if only lru is supported for now) or removing/deferring the parameter until multiple strategies exist.

    CacheIO(const CacheIOParameter& param, Allocator* allocator, Args&&... args)
        : BasicIO<CacheIO<IOTmpl>>(allocator),
          cache_(std::make_unique<LRUPageCache>(std::max(
              uint64_t(1), param.total_cache_size_ / std::max(uint64_t(1), param.page_size_)))),
          inner_io_(std::make_unique<IOTmpl>(std::forward<Args>(args)...)) {
        Page::SetPageSize(std::max(uint64_t(1), param.page_size_));
        this->size_ = inner_io_->size_;

src/io/common/io_parameter.cpp:66

• IOParameter::GetIOParameterByJson now parses type: cache_io, but other runtime IO-type validation/selection paths do not recognize it (e.g., src/factory/factory.cpp:is_valid_streaming_io_type only allows memory/block_memory/buffer/async/mmap/reader). This can lead to configuration that parses successfully but is later rejected or cannot be instantiated. Please ensure all IO factories/validators that consume IOParameter are updated to support cache_io, or avoid advertising it as a JSON-selectable type until it can be constructed end-to-end.

        } else if (type_name == IO_TYPE_VALUE_MMAP_IO) {
            io_ptr = std::make_shared<MMapIOParameter>();
            io_ptr->FromJson(json);
        } else if (type_name == IO_TYPE_VALUE_CACHE_IO) {
            io_ptr = std::make_shared<CacheIOParameter>();
            io_ptr->FromJson(json);
        } else if (type_name == IO_TYPE_VALUE_READER_IO) {

src/io/cache_io/page.h:67

• Page uses a process-wide static page_size_ that is mutated by CacheIO constructors. Creating multiple CacheIO instances with different page_size values (or tests that call Page::SetPageSize) can make existing cached Page buffers and page-id calculations inconsistent, risking out-of-bounds reads/copies.

    static void
    SetPageSize(uint64_t page_size) {
        static std::mutex mtx;
        std::lock_guard<std::mutex> lock(mtx);
        page_size_ = page_size;
    }

    static uint64_t
    PageSize() {
        return page_size_;
    }

src/io/cache_io/cache_io.h:76

• WriteImpl updates the inner IO but never invalidates cached pages. If a caller reads data into the cache, then overwrites the same region via Write, subsequent reads can return stale cached bytes. Either enforce the “no overlapping reads/writes” precondition at runtime or invalidate the affected pages on write.

    void
    WriteImpl(const uint8_t* data, uint64_t size, uint64_t offset) {
        inner_io_->WriteImpl(data, size, offset);
        if (offset + size > this->size_) {
            this->size_ = offset + size;
        }
    }

src/io/cache_io/cache_io.h:117

• DirectReadImpl always allocates/copies and sets need_release=true. This contradicts the PR description that single-page direct reads should return an internal cache pointer with need_release=false. If the intended contract is “always allocate”, the PR description/tests should be updated; if the intended contract is “return cache pointer”, the implementation needs a safe pinning/lifetime strategy to prevent eviction from invalidating the returned pointer.

    [[nodiscard]] const uint8_t*
    DirectReadImpl(uint64_t size, uint64_t offset, bool& need_release) const {
        need_release = false;
        if (size == 0 or not is_valid_range(size, offset)) {
            return nullptr;
        }
        auto* data = reinterpret_cast<uint8_t*>(this->allocator_->Allocate(size));
        if (data == nullptr) {
            return nullptr;
        }
        if (not ReadImpl(size, offset, data)) {
            this->allocator_->Deallocate(data);
            return nullptr;
        }
        need_release = true;
        return data;
    }

src/algorithm/ivf/ivf.cpp:49

• The default IVF JSON template injects CACHE_IO_* fields into IO param blocks even when {TYPE_KEY} is not cache_io (e.g., memory_io, block_memory_io). This makes the template misleading and can cause unrelated IO parameter parsers to receive cache-specific keys. Prefer only including these fields when the IO type is actually cache_io (or nest them under an explicit inner_io/cache_io object) to keep the template self-consistent.

static constexpr const char* IVF_PARAMS_TEMPLATE =

src/io/cache_io/page.h:35

• Page uses a process-wide static page_size_ that is mutated by Page::SetPageSize(). Since Page allocates allocator_->Allocate(page_size_) at construction time, changing the global page size later (e.g., by constructing another CacheIO with a different page_size_) can make existing cached pages too small/large for subsequent reads, risking out-of-bounds reads/writes and memory corruption.

 * All pages share a process-wide uniform page size.
 */
class Page {
public:
    explicit Page(Allocator* allocator) : allocator_(allocator) {
        data_ = static_cast<uint8_t*>(allocator_->Allocate(page_size_));
    }

src/io/cache_io/cache_io.h:100

• WriteImpl does not invalidate cached pages that overlap the written range. If a page was previously read into the cache, a later write to the same region can leave the cache serving stale data on subsequent reads.

    void
    WriteImpl(const uint8_t* data, uint64_t size, uint64_t offset) {
        inner_io_->WriteImpl(data, size, offset);
        if (offset + size > this->size_) {
            this->size_ = offset + size;
        }
    }

src/io/cache_io/cache_io.h:141

• DirectReadImpl always allocates + copies (sets need_release=true) and never returns a pointer into the cache on single-page hits. This contradicts the PR description’s stated behavior for DirectRead, and it also means DirectRead has no zero-copy fast path.

    [[nodiscard]] const uint8_t*
    DirectReadImpl(uint64_t size, uint64_t offset, bool& need_release) const {
        need_release = false;
        if (size == 0 or not is_valid_range(size, offset)) {
            return nullptr;
        }
        auto* data = reinterpret_cast<uint8_t*>(this->allocator_->Allocate(size));
        if (data == nullptr) {
            return nullptr;
        }
        if (not ReadImpl(size, offset, data)) {
            this->allocator_->Deallocate(data);
            return nullptr;
        }
        need_release = true;
        return data;
    }

src/io/cache_io/cache_io.h:62

• CacheIOParameter exposes eviction_strategy_, but CacheIO always constructs LRUPageCache and ignores the strategy setting. If only LRU is supported right now, it would be clearer to either remove the parameter or explicitly document/validate that only "lru" is accepted.

    CacheIO(const CacheIOParameter& param, Allocator* allocator, Args&&... args)
        : BasicIO<CacheIO<IOTmpl>>(allocator),
          cache_(std::make_unique<LRUPageCache>(std::max(
              uint64_t(1), param.total_cache_size_ / std::max(uint64_t(1), param.page_size_)))),
          inner_io_(std::make_unique<IOTmpl>(std::forward<Args>(args)...)) {
        Page::SetPageSize(std::max(uint64_t(1), param.page_size_));
        this->size_ = inner_io_->size_;
    }

src/io/cache_io/page_cache.cpp:50

• PageCache::Insert can spin forever if PickVictim() returns a page_id that isn't present in pages_ (pages_.size() never decreases). It also calls OnRemove() even when nothing was erased, which can desync the eviction state.

    while (pages_.size() >= max_pages_) {
        uint64_t victim = PickVictim();
        if (victim == UINT64_MAX) {
            return page;
        }
        pages_.erase(victim);
        OnRemove(victim);
    }

src/io/cache_io/cache_io.h:114

• ReadImpl uses Options::Instance().cache_page_size() without guarding against 0, which would cause division/modulo by zero when computing page_id/page_offset.

        }
        uint64_t page_size = Options::Instance().cache_page_size();
        uint64_t cur_size = 0;

src/io/cache_io/cache_io.h:192

• get_or_load_page() uses Options::Instance().cache_page_size() without guarding against 0, which can lead to division-by-zero behavior when calculating file_offset.

        uint64_t page_size = Options::Instance().cache_page_size();
        uint64_t file_offset = page_id * page_size;
        if (file_offset >= this->size_) {

src/io/cache_io/cache_io.h:147

• DirectReadImpl currently always allocates + copies into a new buffer and sets need_release=true. This contradicts the PR description/design notes that single-page hits should return an internal cache pointer with need_release=false. Either implement a safe page-pinning mechanism (so the cached page stays alive while the caller uses the pointer) or update the design/docs to match the current always-copy behavior.

    [[nodiscard]] const uint8_t*
    DirectReadImpl(uint64_t size, uint64_t offset, bool& need_release) const {
        need_release = false;
        if (size == 0 or not is_valid_range(size, offset)) {
            return nullptr;
        }
        auto* data = reinterpret_cast<uint8_t*>(this->allocator_->Allocate(size));
        if (data == nullptr) {
            return nullptr;
        }
        if (not ReadImpl(size, offset, data)) {
            this->allocator_->Deallocate(data);
            return nullptr;
        }
        need_release = true;
        return data;
    }

src/datacell/graph_interface.cpp:90

• CacheIOParameter doesn’t define inner_io_parameter_, so this branch won’t compile. Use the parsed string field (inner_io_type_) instead when selecting the CacheIO specialization.

                if (cache_param) {
                    auto inner_type = cache_param->inner_io_type_;

src/datacell/flatten_interface.cpp:310

• CacheIOParameter doesn’t have inner_io_parameter_, so this code won’t compile. Use inner_io_type_ (set from JSON) to choose the CacheIO instantiation.

        auto cache_param = std::dynamic_pointer_cast<CacheIOParameter>(param->io_parameter);
        if (cache_param) {
            auto inner_type = cache_param->inner_io_type_;

src/datacell/bucket_interface.cpp:120

• CacheIOParameter doesn’t define inner_io_parameter_, so this code won’t compile. Use inner_io_type_ to determine which CacheIO specialization to build.

    if (io_type_name == IO_TYPE_VALUE_CACHE_IO) {
        auto cache_param = std::dynamic_pointer_cast<CacheIOParameter>(param->io_parameter);
        if (cache_param) {
            auto inner_type = cache_param->inner_io_type_;
            if (inner_type == IO_TYPE_VALUE_MMAP_IO) {

src/io/cache_io/cache_io.h:95

• CacheIO(io_param, common_param) doesn’t validate inner_io_type_. If it’s missing/invalid, GetIOParameterByJson can return nullptr and the inner IO construction will fail later without a clear error. Validate inner_io_type_ and the returned inner_io_param and throw a helpful exception early.

        JsonType inner_json = cache_param->original_json_;
        inner_json[TYPE_KEY].SetString(cache_param->inner_io_type_);
        auto inner_io_param = IOParameter::GetIOParameterByJson(inner_json);
        inner_io_ = std::make_unique<IOTmpl>(inner_io_param, common_param);
        this->size_ = inner_io_->size_;

src/io/cache_io/cache_io.h:78

• CacheIO uses Options::Instance().cache_page_size() as mutable global state (including setting it in the ctor) and also relies on it for page indexing and copies. If multiple CacheIO instances are created with different page sizes, or cache_page_size is changed after pages are allocated, reads can compute offsets/copy sizes with a different page size than Page allocated, risking out-of-bounds access.

                           page_size > 0 ? page_size : Options::Instance().cache_page_size())))),
          inner_io_(std::make_unique<IOTmpl>(std::forward<Args>(args)...)) {
        if (page_size > 0) {
            Options::Instance().set_cache_page_size(page_size);
        }

src/io/cache_io/cache_io.h:135

• PR description says DirectRead should return an internal cache pointer on a single-page hit with need_release=false, but DirectReadImpl always allocates/copies and sets need_release=true. Either update the implementation to match the stated design (including safe lifetime/pinning semantics) or adjust the PR description/tests to reflect the actual behavior.

    [[nodiscard]] const uint8_t*
    DirectReadImpl(uint64_t size, uint64_t offset, bool& need_release) const {
        need_release = false;
        if (size == 0 or not is_valid_range(size, offset)) {
            return nullptr;

src/algorithm/hgraph/hgraph_param_mapping.cpp:199

• HGraph exposes cache_page_size parameters (and constants.h declares HGRAPH__CACHE_PAGE_SIZE), and examples/tests set base_cache_page_size/precise_cache_page_size, but map_hgraph_param() doesn’t map any of those external keys into IO_PARAMS_KEY/CACHE_IO_PAGE_SIZE_KEY. As a result, user-provided page sizes are silently ignored and only the template default (131072) applies.

            HGRAPH_BASE_CACHE_TOTAL_SIZE,
            {
                BASE_CODES_KEY,
                IO_PARAMS_KEY,
                CACHE_IO_TOTAL_CACHE_SIZE_KEY,

src/algorithm/ivf/ivf.cpp:143

• IVF exposes base_cache_page_size/precise_cache_page_size (constants.h declares IVF_*_CACHE_PAGE_SIZE), but CheckAndMappingExternalParam only maps cache total size + inner io type. External page size overrides are never mapped into CACHE_IO_PAGE_SIZE_KEY, so user-provided page sizes are ignored.

            IVF_BASE_CACHE_TOTAL_SIZE,
            {
                BUCKET_PARAMS_KEY,
                IO_PARAMS_KEY,
                CACHE_IO_TOTAL_CACHE_SIZE_KEY,

src/algorithm/ivf/ivf.cpp:1

• The default IO params template includes CACHE_IO_TOTAL_CACHE_SIZE_KEY while the IO {TYPE_KEY} is not cache_io (here it’s memory_io). If IO parameter parsing is strict (or becomes strict later), these extra keys can become a compatibility hazard, and even if parsing is permissive it makes the templates harder to reason about (keys that don’t apply to the selected type). Consider only emitting cache-specific keys when {TYPE_KEY} is cache_io, or nesting them under a cache-io-specific object so the meaning is unambiguous.